-
-
Notifications
You must be signed in to change notification settings - Fork 404
Return error on out of space #1504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
29895f0 to
60bc8f2
Compare
|
looks good ! could you add yourself to the AUTHORS file ? |
60bc8f2 to
90f0a3d
Compare
|
Done! ls the added coverage enough? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1504 +/- ##
==========================================
- Coverage 74.90% 74.48% -0.43%
==========================================
Files 160 160
Lines 18555 18592 +37
==========================================
- Hits 13899 13848 -51
- Misses 3504 3592 +88
Partials 1152 1152 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
949e41f to
eac71af
Compare
|
I think there is something wrong. you added tests which actually should cover the changes.. but: patch coverage is 27.90698% and in the diff view the lines are red (needs codeov plugin installed in browser now) ... |
|
|
also it did not error |
eac71af to
1e7c15b
Compare
|
The test env does not have root priviledges... so the relevant tests are skipped: https://github.com/aptly-dev/aptly/actions/runs/20895944097/job/60034478958?pr=1504#step:8:652 That's why the coverage is low... let's see if there is another way to test this. |
i never fully understood why coverage is sometimes shown and sometimes not. |
|
maybe we can try running these tests with sudo |
Actually I think this missing codecov token is expected. this is running on my fork, which does not have access to the secret |
|
and now the codecov check is missing? what's happening? |
|
actually nevermind I think everything should work now. @neolynx if you create a PR to check I think codecov should look better now! |
|
codecov/patch — 75.55% of diff hit (target 74.90%) way better... ;-) let me update the PR |
00db86b to
d035505
Compare
|
I added 3 changes:
now, when not running as root, a /smallfs mount is expected. Pipeline and make docker-unit tests start a docker container providing a 1MB /smallfs... maybe the coverage upload should fail the pipeline if it cannot upload ? |
- run separate unit-test job - build docker - allow make docker-unit-tests in ci
d035505 to
9a6f06d
Compare
8b970d5 to
9c9c608
Compare
|
coverage is not separate job, can be restarted more easily :) |
|
thanks! I see the codecov run failed, and it failed CI.
we could try using the latest codecov action v5 and see if that helps. (separately: do we not have an automatic dependency bumper like Dependabot configured for this repo, so that this would have been updated already?) Also sounds like uploading without a token is expected to fail, so I'm not sure if we want this to fail CI on PRs from forks like this, since it will fail, and it's expected to fail. |
Fixes #1474
Requirements
All new code should be covered with tests, documentation should be updated. CI should pass.
Description of the Change
Return an error if we run out of disk space. Batch up the sync at the end of writing all files.
Checklist
AUTHORS